Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shift responsibility of IDFA collection to clients #5

Merged
merged 1 commit into from
Oct 3, 2020

Conversation

sjmadsen
Copy link
Contributor

@sjmadsen sjmadsen commented Oct 2, 2020

What does this PR do?
This is a backport from segmentio/analytics-ios (segmentio/analytics-ios#892).

Any references to Apple's AdSupport framework might trip the App Store's static analysis, making it a bit unclear if SDK users can confidently say that their apps do not use the IDFA. By moving the responsibility to collect the IDFA out of the PostHog SDK and into the client app, the AdSupport references are gone and the question is easily answered.

Where should the reviewer start?
It's a pretty small PR, but note that the PHGIDFA() function is gone and the caller of that function now checks for and calls a block provided as part of the SDK initialization.

How should this be manually tested?
With no changes to a client app, trigger an event, and verify that $device_advertisingId no longer has any value.

Then modify the app to provide a block for adSupportBlock in the PostHog configuration. Triggering an event should now include the return value from this block as the value for $device_advertisingId.

Any background context you want to provide?

What are the relevant tickets?
#4 - Remove references to AdSupport framework

Screenshots or screencasts (if UI/UX change)
n/a

Questions:

  • Does the docs need an update? Yes; if apps want to collect the IDFA from the user's device, merely setting enableAdvertisingCapturing is no longer sufficient.
  • Are there any security concerns? No.
  • Do we need to update engineering / success? No?

This is a backport from segmentio/analytics-ios (segmentio/analytics-ios#892).

Any references to Apple's AdSupport framework might trip the App Store's static analysis, making it a bit unclear if SDK users can confidently say that their apps do not use the IDFA. By moving the responsibility to collect the IDFA out of the PostHog SDK and into the client app, the AdSupport references are gone and the question is easily answered.
@timgl
Copy link

timgl commented Oct 2, 2020

@sjmadsen Awesome, thanks so much for contributing this.

@mariusandra would you mind reviewing this?

@mariusandra
Copy link
Contributor

Thank you @sjmadsen , this is awesome!

Could you just also provide an example that we could put in the docs for users who want to use this AdSupport framework? Preferably for both Objective-C and Swift...

@sjmadsen
Copy link
Contributor Author

sjmadsen commented Oct 2, 2020

I sure can. It's probably worth calling out separately from the "all configuration options" section now, since it's not enough to use the defaults and still get a value from clients for $device_advertiserId.

To capture the IDFA in Objective-C, SDK clients will want to do something like this:

// At the top of your .m file
@import AdSupport;  // or #import <AdSupport/ASIdentifierManager.h>

// During PostHog initialization
PHGPostHogConfiguration` *configuration = [PHGPostHogConfiguration configurationWithApiKey:@"YOUR_API_KEY"];
configuration.enableAdvertisingCapturing = YES;
configuration.adSupportBlock = ^{
    return [[ASIdentifierManager sharedManager].advertisingIdentifier uuidString];
};

Or in Swift:

// At the top of your .swift file
import AdSupport

// During PostHog initialization
let configuration = PHGPostHogConfiguration(apiKey: "YOUR_API_KEY")
configuration.enableAdvertisingCapturing = true
configuration.adSupportBlock = {
    return ASIdentifierManager.shared().advertisingIdentifier.uuidString
}

@mariusandra mariusandra merged commit 7fb57d3 into PostHog:master Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants